-
-
Notifications
You must be signed in to change notification settings - Fork 933
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open api support #1050
Open api support #1050
Conversation
How does this differ from #1046? |
Hi @ryu-0729 @Indyandie, you are working to achieve the same objective on different pull requests. It's better to join forces and work together on a single one. |
Thanks @Naramsim, agreed. @ryu-0729 I'll need to review to see what are the major differences. Can you push the generated spec? It will be simpler to review and I can even run a diff between our specs.
The big one at first glance is that I did not implement the Swagger and ReDoc UI generation. I personally think that should be handle in pokemonapi.co once we have the openAPI in good shape. |
@Indyandie Thanks for your comment.
I agree with you. I have pushed the generated OpenAPI definitions and would appreciate your review. |
Ideally yes, it would be cool to have a swagger browsable at pokeapi.co. The thing is that pokeapi.co is a static website made with React. And there's no Django service behind it. The API is literally a series of JSONs scraped from the Django server: https://github.com/PokeAPI/api-data So it would be best to use something like this to publish the swagger to pokeapi.co: https://swagger.io/docs/open-source-tools/swagger-ui/customization/overview/ or https://github.com/swagger-api/swagger-ui Nonetheless, having the SpectacularSwaggerView working locally when building locally is really nice to have. I'd prioritize on this former one, and later think about the one we can integrate at pokeapi.co |
@ryu-0729 @Naramsim I compared our OpenAPi documents by previewing them with stoplight elements. It looks like we did a lot of duplicate work. The big differences are that my PR has some additional info. For example summary, descriptions, tags, and examples. This additional info helps consumers, for instance including a tag will group related endpoints. I think we should merge my PR (if everything checks out) and repurpose this PR to handle the doc generation related to @Naramsim last comment. I would love to help with that. I'll post some feedback another time. |
Hi, I took a look at both the PRs. I like Vice versa, I like Now, If I have to be honest I thought that the OpenAPI schema could have been inferred from the code. Did you really write by hand those It's true that the API doesn't change that often, but still this will require that at each schema modification we should modify also the |
@Naramsim thanks for taking a closer look. Is there a simple way we can merge our branches? I'll speak for myself but yes I wrote it all by hand. And I did have to do some investigations, calling the endpoints and reviewing the CSVs. It was a little intense but luckily I really like writing docs. Yes, we will need to keep some of the definitions updated, but once we're able to implement the OpenAPI doc into the site we'll reap the benefits. I'm a total noob when it comes to the drf-spectacular but I'm hopeful once smarter folks get familiar with the tool they find ways to improve automation. |
@Naramsim
Basically, it guesses from the code and creates the schema for you. It is also possible to simplify by defining a serializers class for formatting. |
Hi, this commit seems so clean: e20e3f8! I'm a bit sick and cannot review/decide which version to go to now @ryu-0729 @Indyandie. If we merge one of the PRs we would also need some support in the coming time in case we need to change/update the code. Will you be around and available if we ping you? |
@Naramsim Sorry to hear that, I hope you feel better soon. Yes I should be around, I'm usually available after 05:00 UTC. |
@Naramsim
Of course! We will support you. |
Hi! I merged the other PR! I have to close this even if you did a great job :( If you are still available you could improve the implementation of @Indyandie or contribute in a different way :) |
Hello.
I am not good at English, so sorry if I am rude.
We have added OpenAPI support and UI for Swagger and ReDoc.
You will be able to try the API on the web.
You can also visualize response and request types.
Swagger: http://localhost/api/v2/schema/swagger-ui/
ReDoc: http://localhost/api/v2/schema/redoc/
Some changes are convered by #1046 pull request.
Relevant issue: #528